Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

【Hackathon 7th No.31】NO.31 为 paddle.sparse.sparse_csr_tensor进行功能增强 #68281

Merged
merged 3 commits into from
Sep 27, 2024
Merged

【Hackathon 7th No.31】NO.31 为 paddle.sparse.sparse_csr_tensor进行功能增强 #68281

merged 3 commits into from
Sep 27, 2024

Conversation

monster1015
Copy link
Contributor

@monster1015 monster1015 commented Sep 18, 2024 •
edited by zhwesky2010
Loading

PR Category

User Experience

PR Types

Improvements

Description

【Hackathon 7th No.31】NO.31 为 paddle.sparse.sparse_csr_tensor进行功能增强

@paddle-bot Paddle-bot
Copy link

paddle-bot bot commented Sep 18, 2024

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@CLAassistant
Copy link

CLAassistant commented Sep 18, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@luotao1 luotao1 mentioned this pull request Sep 18, 2024
Open
@monster1015
Copy link
Contributor Author

@luotao1 ，请问这个ci全说我未验证是怎么回事？

@luotao1
Copy link
Contributor

luotao1 commented Sep 18, 2024

请问这个ci全说我未验证是怎么回事？

@monster1015 已处理，可以重新提交一个commit

zhwesky2010
zhwesky2010 reviewed Sep 18, 2024
View reviewed changes
python/paddle/sparse/creation.py Outdated
@@ -199,13 +199,25 @@ def sparse_coo_tensor(
return out


def _infer_dense_csr_shape(crows, cols, values):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该没这么容易，3D的推导没有支持

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对的，根据我查找的资料，CSR稀疏矩阵应该是2维，我是准备给他改掉的。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhwesky2010

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我检查了一下，是我搞错了，请稍等

@monster1015
Copy link
Contributor Author

image
@luotao1 这个好像说需要您这里检查

@monster1015
Copy link
Contributor Author

image
请问这是怎么回事？ @luotao1

@luotao1
Copy link
Contributor

luotao1 commented Sep 19, 2024

  1. 覆盖率Coverage是确定性错误，为了节省CI资源，禁止Rerun
  2. 需要approve的地方，最后会统一处理

@monster1015
Copy link
Contributor Author

  1. 覆盖率Coverage是确定性错误，为了节省CI资源，禁止Rerun
  2. 需要approve的地方，最后会统一处理

那我现在是不用管他了？

@luotao1
Copy link
Contributor

luotao1 commented Sep 19, 2024

需要增加单侧来通过覆盖率测试

@monster1015
Copy link
Contributor Author

需要增加单侧来通过覆盖率测试

好的好的

@monster1015
Copy link
Contributor Author

需要增加单侧来通过覆盖率测试

已经添加了

@monster1015
Copy link
Contributor Author

PR-CI-Static-Check这个好像是需要你们那里approve的 @luotao1

@luotao1
Copy link
Contributor

luotao1 commented Sep 20, 2024

需要approve的地方，review通过后会统一处理

@monster1015
Copy link
Contributor Author

那要求的required已经全都跑完了的

zhwesky2010
zhwesky2010 reviewed Sep 23, 2024
View reviewed changes
python/paddle/sparse/creation.py Outdated
@@ -199,13 +199,30 @@ def sparse_coo_tensor(
return out


def _infer_dense_csr_shape(crows, cols):
crows_numpy = crows.numpy()
batchs = np.sum(crows_numpy[:-1] > crows_numpy[1:])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是不是应该是再加一个1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对的对的，返回那里要加的

test/legacy_test/test_sparse_utils_op.py
@@ -125,6 +125,16 @@ def test_create_coo_no_shape(self):
coo = paddle.sparse.sparse_coo_tensor(indices, values)
assert [2, 2] == coo.shape

def test_create_csr_no_shape(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

单测多测几个case吧，测全一点，3D至少测两个

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的好的

zhwesky2010
zhwesky2010 reviewed Sep 24, 2024
View reviewed changes
test/legacy_test/test_sparse_utils_op.py Outdated
crows = paddle.to_tensor(crows, dtype='int32')
clos = paddle.to_tensor(clos, dtype='int32')
values = paddle.to_tensor(values, dtype='float32')
coo = paddle.sparse.sparse_csr_tensor(crows, clos, values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

csr

test/legacy_test/test_sparse_utils_op.py Outdated
crows = paddle.to_tensor(crows, dtype='int32')
clos = paddle.to_tensor(clos, dtype='int32')
values = paddle.to_tensor(values, dtype='float32')
coo = paddle.sparse.sparse_csr_tensor(crows, clos, values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

csr

test/legacy_test/test_sparse_utils_op.py Outdated
crows = paddle.to_tensor(crows, dtype='int32')
clos = paddle.to_tensor(clos, dtype='int32')
values = paddle.to_tensor(values, dtype='float32')
coo = paddle.sparse.sparse_csr_tensor(crows, clos, values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

csr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已经好了

zhwesky2010
zhwesky2010 approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@luotao1 luotao1 merged commit d2744a0 into PaddlePaddle:develop Sep 27, 2024
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhwesky2010 zhwesky2010 zhwesky2010 approved these changes

Assignees

@luotao1 luotao1

@zhwesky2010 zhwesky2010

Labels
contributor External developers PaddlePaddle Hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@monster1015 @CLAassistant @luotao1 @zhwesky2010